Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(es/modules): Preserve extensions #6339

Merged
merged 8 commits into from
Nov 3, 2022
Merged

Conversation

kdy1
Copy link
Member

@kdy1 kdy1 commented Nov 3, 2022

@kdy1 kdy1 added this to the Planned milestone Nov 3, 2022
@kdy1 kdy1 self-assigned this Nov 3, 2022
@kdy1 kdy1 marked this pull request as ready for review November 3, 2022 03:37
kodiakhq[bot]
kodiakhq bot previously approved these changes Nov 3, 2022
@kdy1 kdy1 enabled auto-merge (squash) November 3, 2022 03:37
Copy link
Member Author

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swc-bump:

  • dbg-swc

kodiakhq[bot]
kodiakhq bot previously approved these changes Nov 3, 2022
@kdy1 kdy1 marked this pull request as draft November 3, 2022 03:52
auto-merge was automatically disabled November 3, 2022 03:52

Pull request was converted to draft

@kdy1 kdy1 marked this pull request as ready for review November 3, 2022 05:06
@kdy1 kdy1 enabled auto-merge (squash) November 3, 2022 05:06
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was workaround for ts, i.e.

import foo from "./test.ts"

to

import foo from "./test"

So test.js can be resolved, not sure why it was implemented in this way 😕

babel keeps exetensions too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤣 I made lots of mistakes

Copy link
Collaborator

@swc-bot swc-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review comment generated by auto-rebase script

@kdy1 kdy1 merged commit 91e863c into swc-project:main Nov 3, 2022
@kdy1 kdy1 deleted the issue-6209 branch November 3, 2022 07:04
@kdy1 kdy1 modified the milestones: Planned, v1.3.15 Nov 12, 2022
@Nickersoft
Copy link

Hey @kdy1! I just had to downgrade because of this change, for the exact reason I think you mentioned in your comment 😅 I have a NestJS project where none of my imports use extensions (as per NestJS convention) – however, now my TypeScript compile from this:

import foo from './bar.module';

to

import foo from './bar.module.ts';

even though SWC is generating a directory full of JavaScript files. My code crashes because it can't find bar.module.ts (because it's compiled to bar.module.js). Even changing my code to import from bar.module.js (the weird ESM convention for TS), SWC still generates the import with the .ts file extension lol.

Is there any workaround for this? Or am I stuck on 1.3.14 for now?

@kdy1
Copy link
Member Author

kdy1 commented Nov 13, 2022

Use .js for omports instead. swc is not a fixer. Your code is wrong. swc should not fix it.

@kdy1
Copy link
Member Author

kdy1 commented Nov 13, 2022

You can fix your code. It's not a something to workaround. Your code is simply wrong.

@Nickersoft
Copy link

Nickersoft commented Nov 14, 2022

As I mentioned, .js imports get rewritten back to .ts anyway.

I spent some time looking into this, and realized that as long as you have a paths option in .swcrc, it will break imports.

I set up this example repo.

The project is ESM, and with a main.ts file that imports a TypeScript file (import.ts) using a .js extension. There is a bogus paths entry in .swcrc (doesn't matter if it's valid or not – I checked). Run npm install and then npm run build. It'll compile to a dist directory, but the code will be un-runnable because the .js import was rewritten back to a .ts import.

This is a legitimate problem, regardless of how you're importing your modules.

@kdy1
Copy link
Member Author

kdy1 commented Nov 14, 2022

My bad, I misread your comment, sorry. Can you file an issue?

@Nickersoft
Copy link

Yep 👍🏻

@swc-project swc-project locked as resolved and limited conversation to collaborators Dec 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Package imports are rewritten incorrectly if they have 3 or more segments separated by dots
4 participants